Use content digest for pulled service images - #13998
Conversation
pullServiceImage returned the pulled image's raw inspect ID, while getImageSummaries resolves already-local images through contentDigest (the platform image-manifest digest). Both values feed the com.docker.compose.image label that mustRecreate compares to detect image changes, so the two paths disagreeing made the first 'up' after the pulling 'up' see a phantom image change and recreate every container once, with no change anywhere. Under the containerd image store a tag@digest reference triggers this: the raw inspect ID is the index digest, while contentDigest picks the platform manifest digest. Resolve the pulled image through the same manifests-aware inspect and contentDigest call getImageSummaries uses, so both sides of the staleness comparison speak the same scheme. Verified against a fresh docker:dind (29.7.0, containerd store) with a tag@digest service: unpatched v5.4.0 recreates the container on the second 'up'; with this fix the container survives repeated 'up' runs. Existing behavior is preserved for engines without manifest support (contentDigest falls back to the plain ID). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Max Malm <benjick@dumfan.net>
Extract the inspect-with-manifests + contentDigest sequence from pullServiceImage into inspectContentDigest, next to the contentDigest and manifestsSupported helpers it belongs with. pullServiceImage was over the gocyclo limit of 16 with the resolve block inlined, and getImageSummaries already ran the same sequence, so this names it once. Rename the image parameter of the serviceWithHook test helper to img: it shadowed the moby image package, newly imported by pull_test.go. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Max Malm <benjick@dumfan.net>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
Hi @benjick, thanks for this fix and for the detailed analysis, the pull-path/local-path digest disagreement you identified was exactly right. While reviewing it we realized it's one instance of a broader structural problem: compose records image identities through several independent code paths (pull, bake, classic builder, already-local inspect), and they don't all produce the same kind of digest for the same image. The same mismatch class caused #13636 and #14005, plus a few more latent cases we found along the way. We've addressed the whole class in #14011: a single canonical content-digest producer shared by every path, plus a containerd-store e2e job in CI so this can't regress silently again. Your commit is included there with your authorship preserved (squashed with your lint follow-up), and your If that works for you, we'll close this PR in favor of #14011 , and feel free to review it there, your feedback would be very welcome. Thanks again! |
|
Yeah 100%! |
What I did
pullServiceImagereturned the pulled image's raw inspect ID, whilegetImageSummariesresolves already-local images throughcontentDigest(the platform image-manifest digest, introduced for #13636). Both values feed thecom.docker.compose.imagelabel thatmustRecreatecompares to detect image changes - so the two paths disagreeing made the firstupafter the pullingupsee a phantom image change and recreate every container once, with no change to the project, the image, or the compose file.Under the containerd image store a
tag@digestimage reference triggers this reliably: the pull path records the index digest (the raw inspect ID) on the container, and the nextupresolves the same image to its platform manifest digest viacontentDigest. The strings differ,mustRecreatefires, and the replacement container gets the platform digest - so it happens exactly once per container, which makes it easy to miss but breaks any workflow that relies onupbeing idempotent (we hit it because our orchestrator asserts unchanged services are never recreated across reconciles).The fix makes
pullServiceImageresolve the pulled image through the same manifests-aware inspect +contentDigestcallgetImageSummariesuses, so both sides of the staleness comparison speak the same scheme. Engines without manifest support (API < 1.48) keep the previous behavior viacontentDigest's plain-ID fallback.Repro
The two containers'
com.docker.compose.imagelabels show the mismatch: container A carriessha256:6baf...(the pinned index digest), its replacement carries the platform manifest digest. v5.3.1 does not exhibit this; it appeared when the local-inspect side moved tocontentDigest.Verification
TestPullServiceImageUsesContentDigestfails against the old return value and passes with the fix.go build ./...,go vet ./pkg/compose/,gofmt -s -lclean;go test ./pkg/compose/passes.docker:dind(29.7.0): with this fix the container from the pullingupsurvives repeatedupruns (same container ID); unpatched v5.4.0 recreates it on the secondup.Related: #13636 (the fix that introduced
contentDigestfor the local-inspect path; this aligns the pull path with it).Disclosure: this PR was prepared by Claude Code (Anthropic) - the diagnosis, fix, tests, and the verification runs above - in an agentic session I directed. I'm the submitter and point of contact.
🤖 Generated with Claude Code